Skip to content

odb: fix 3dblox declaration mismatches #9179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
osamahammad21 merged 2 commits into from
Jan 4, 2026
Merged

odb: fix 3dblox declaration mismatches #9179

osamahammad21 merged 2 commits into from
Jan 4, 2026

Conversation

@ahmed532
Copy link

@ahmed532 ahmed532 commented Dec 31, 2025

Some structs in src/odb/include/odb/3dblox.h where forward declared as classes. Which errored with "-Wmismatched-tags" compiler flag. Also, found another mismatch in PartitionMgr.h
Note that C++ does not distinguish between class and struct in a forward declaration. Only MSVC can have issues with it.

odb: fix 3dblox declaration mismatches
fe55901 
Some structs in src/odb/include/odb/3dblox.h where forward declared
as classes. Which errored with "-Wmismatched-tags" compiler flag.
Also, found another mismatch in PartitionMgr.h
Note that C++ does not distinguish between class and struct in a
forward declaration. Only MSVC can have issues with it.

Signed-off-by: Ahmed R. Mohamed <[email protected]>
gemini-code-assist[bot]
gemini-code-assist bot reviewed Dec 31, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly fixes mismatched forward declarations of structs that were declared as class, which resolves -Wmismatched-tags compiler warnings. The changes in src/odb/include/odb/3dblox.h and src/par/include/par/PartitionMgr.h are accurate and improve code correctness by ensuring consistency between declarations and definitions. The grouping of struct forward declarations in 3dblox.h also slightly improves readability. The changes look good to me.

@ahmed532 ahmed532 self-assigned this Dec 31, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 31, 2025

clang-tidy review says "All clean, LGTM! 👍"

@maliberty
Copy link
Member

maliberty commented Dec 31, 2025

Apparently it matters on mac as well which doesn't use MSVC.

@ahmed532
Copy link
Author

ahmed532 commented Dec 31, 2025

Apparently it matters on mac as well which doesn't use MSVC.

OK I'll add it to compile options for clang then.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 31, 2025

clang-tidy review says "All clean, LGTM! 👍"

@ahmed532
Copy link
Author

ahmed532 commented Dec 31, 2025

This started with #9093 which was reported by Stefan who uses a mac. To my knowledge we have zero MSVC users (nobody has ever reported an issue). In the report it is a warning but we should clean it up. In the CI we treat warnings as errors to keep them from creeping in.
I don't think you need to make it always an error as non-devs should be able to build even if we have a mismatch
option(ALLOW_WARNINGS "Flag to allow compilation with compiler warnings: on by default" ON)

if (NOT ALLOW_WARNINGS)
  add_compile_options(
    $<$<CXX_COMPILER_ID:GNU>:-Werror>
  )
endif()

in src/CMakeLists.txt
in the CI ALLOW_WARNINGS=0

Quote by @maliberty

@osamahammad21 osamahammad21 merged commit e95d33b into The-OpenROAD-Project:master Jan 4, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@osamahammad21 osamahammad21 osamahammad21 approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ahmed532 ahmed532

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ahmed532 @maliberty @osamahammad21